Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement SpecifiedTradeAllowanceCharges on TradeLineItems for UBL #712

Merged
merged 7 commits into from
Apr 7, 2025

Conversation

lukasschachner
Copy link
Contributor

No description provided.

lukasschachner and others added 5 commits April 3, 2025 09:38
Introduced handling for specified trade allowance charges in trade line items, including new XML elements for allowance details.
Updated handling of optional fields for trade allowance charges to ensure proper checks and formatting. Added conditional logic for ChargePercentage and BasisAmount fields to avoid unnecessary serialization of null values. Improved readability and consistency by using helper methods for enum descriptions.
@stephanstapel
Copy link
Owner

Thanks for the pull request.
Can you please use GetSpecifiedTradeAllowances() and GetSpecifiedTradeCharges() instead ?

Replaced `GetSpecifiedTradeAllowanceCharges` with `GetSpecifiedTradeCharges` to align with naming conventions. Updated reason code serialization method from `GetDescriptionAttribute` to `EnumToString` for improved clarity and consistency.
@lukasschachner
Copy link
Contributor Author

Thanks for the pull request. Can you please use GetSpecifiedTradeAllowances() and GetSpecifiedTradeCharges() instead ?

Just saw it after i merged my branch, was about 30 commits behind.

Extracted redundant logic into a new private method `_WriteItemLevelSpecifiedTradeAllowanceCharge` to improve code readability and maintainability. Simplified loop structure by reusing this method for both trade allowances and trade charges.
@stephanstapel stephanstapel merged commit 00888aa into stephanstapel:master Apr 7, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants